Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow various types to interpolate into string(take 2!!) #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

refi64
Copy link

@refi64 refi64 commented Aug 3, 2014

I think I got everything right. Again, I apologize for the removal of indentation.

@refi64
Copy link
Author

refi64 commented Aug 4, 2014

Bump?

@vadz
Copy link
Collaborator

vadz commented Aug 4, 2014

Sorry, I didn't have time to look at it yet, will try to do it a.s.a.p. (but ideally it should really be @vslavik who should look at it, I still don't know all this code that well myself...).

Where does !! syntax come from BTW?

@vslavik
Copy link
Owner

vslavik commented Aug 5, 2014

Bump?

Really, after only 23 hours?

I apologize for the removal of indentation.

You can always discard or unstage parts of the diff with git… But removing trailing whitespace that shouldn't be there in the first place and is in the code you touch is perfectly fine and appreciated.

@vslavik
Copy link
Owner

vslavik commented Aug 5, 2014

At a glance, this code still won't work with references lists (or other values) that have conditionally set values.

Basically, it's equivalent to #34 as far as I can tell, but expressed a bit differently. My point was that references within interpolated strings need to be handled differently from "outside" references and their concatenations. This patch still treats them the same (case in point: there's no difference between ReferenceAsStringExpr and ReferenceExpr; to_string() seems to be (mostly?) identical to as_py() as well.

@refi64
Copy link
Author

refi64 commented Aug 7, 2014

Ack...I thought ConcatExpr was only used when strings were interpolated. I'm checking to see where it actually occurs now.

@refi64
Copy link
Author

refi64 commented Aug 7, 2014

@vslavik And...I'm lost. Could you point me to where variables get interpolated? I've grepped for ReferenceExpr, 'def reference(', and get_value, all of which have returned nothing useful.

@vslavik
Copy link
Owner

vslavik commented Aug 8, 2014

Could you point me to where variables get interpolated?

They aren't now, because the current grammar treats this case as concatenation of literals with ordinary variable reference. That's why I wrote in in #34 that it needs to be treated specially, with a dedicated Expr specialization that doesn't do just a lookup of a variable, but also converts it to a string (note: "string" in the sense of an Expr tree of StringType (i.e. can be concatenated with other literals), not in the sense of a Python string).

@refi64
Copy link
Author

refi64 commented Aug 10, 2014

Ok...I think I've got something working. It rejects normal concatenation between different types but specializes string interpolation. The only problem now is that I think it set something off that's causing stuff like $(toolset) == gnu to goof up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants